Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prefill with business name and type if they exist, change email domains #177

Merged
merged 6 commits into from
Jul 15, 2024

Conversation

jojo-stripe
Copy link
Collaborator

Disputes now have a customer
image

Business name and type are prefilled if they are specified:

Individual (taken straight to identity info page):
https://github.com/user-attachments/assets/b32e74af-db84-4c13-b764-a118f349055f
Other (have to pick)
https://github.com/user-attachments/assets/7f845753-d4b0-446d-a8bd-a88896b6e6da

Also tested with company but did not screen record

Copy link
Collaborator

@jorgea-stripe jorgea-stripe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes LGTM, left one suggestion

@@ -63,6 +63,7 @@ export async function POST() {
},
confirm: true,
payment_method: 'pm_card_createDispute',
receipt_email: '[email protected]',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see other fixes are bundled here, that's ok since they are very small (though I prefer separate changes) but would be good to mention in the PR description!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The picture in the description is a reference to this change!

@@ -269,7 +269,7 @@ export default function BusinessDetailsForm({email}: {email: string}) {
const form = useForm<z.infer<typeof formSchema>>({
resolver: zodResolver(formSchema),
defaultValues: {
businessType: 'independent_salon',
businessType: 'individual',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the initial state right?

Copy link
Collaborator Author

@jojo-stripe jojo-stripe Jul 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, it's a check box and looks like
image

lib/auth.ts Outdated
@@ -381,10 +382,23 @@ export const authOptions: AuthOptions = {
console.log('Could not find an existing user for the email', email);
return null;
}
let businessType = undefined;

if (
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works, but would be good to do with a switch statement, and to leverage a union type that has the possible values of businessType, to ensure we are handling all cases. Like so:

switch (credentials?.businessType) {
  case 'company':
  case 'individual':
   return credentials?.businessType;
  default:
    assertUnreachable(credentials?.businessType);
    return 'individual'; // We default to individual, but rely on TS to catch missing cases
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added!

lib/auth.ts Outdated

function getRandomInt(min: number, max: number) {
return Math.floor(Math.random() * (max - min + 1)) + min;
}

function assertUnreachable(option: string) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be typed with never, so that the switch statement gives a compile time error when there is a new option

@jojo-stripe jojo-stripe merged commit 5e06b40 into master Jul 15, 2024
2 checks passed
@jojo-stripe jojo-stripe deleted the jojo/fixes branch July 15, 2024 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants